Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Build.ps1: Use minimal distribution instead of standard distribution #57

Closed
wants to merge 1 commit into from
Closed

Build.ps1: Use minimal distribution instead of standard distribution #57

wants to merge 1 commit into from

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Dec 3, 2017 •
edited
Loading

Changes:
Download the minimal distribution instead of standard distribution to speed up the total build time

Resolves #56

@perlun
Copy link
Member

perlun commented Dec 3, 2017

Thanks. @merceyz, have you tested building with this change and verified that it works as expected?

@merceyz
Copy link
Member Author

merceyz commented Dec 3, 2017 •
edited
Loading

No, i'm unable to successfully run the build script due to me having VS2017.

Also before this PR is merged the CEF version needs to be updated

@perlun
Copy link
Member

perlun commented Dec 3, 2017

Also before this PR is merged the CEF version needs to be updated

True. So let's wait on it a while. Thanks anyway, a good start!

@japj
Copy link

japj commented Dec 3, 2017

I can confirm that it does not work yet.

The problem is that the CMakeList.txt contains lines with add_subdirectory(tests/xxxx) and those testclient sources are not present in the minimal distribution. As a result the CMake generation fails. So we probably need to patch the CMakeList.txt before running it.

@perlun
Copy link
Member

perlun commented Dec 3, 2017

I can confirm that it does not work yet.

That's interesting actually! I used a similar approach in my #62 hackish branch, and it did actually work using the minimal distrib. (note: x86 only, I haven't yet built CEF successfully on x64...)

@merceyz
Copy link
Member Author

merceyz commented Dec 3, 2017

The problem is that the CMakeList.txt contains lines with add_subdirectory(tests/xxxx) and those testclient sources are not present in the minimal distribution.

It was fixed in https://bitbucket.org/chromiumembedded/cef/commits/cd6b88f7e4bd455445a7619824c2109904be97c5

Which is why I said

Also before this PR is merged the CEF version needs to be updated

@japj
Copy link

japj commented Dec 4, 2017

Ah, ... CEF was updated 3 days ago to fix this very issue. That is a “recent” version indeed ;)

perlun added a commit that referenced this pull request Dec 16, 2017
@perlun
Upgrade to CEF 63
e1511bd 
I noted that the CEF 63 binaries are now published. We might as well start doing some of the plumbing for an eventual CefSharp 63.0.0 release.

(The README.txt diff is somewhat more noisy than expected, since I copied in the file from the minimal distribution instead. We are migrating to use that: #57)
@perlun perlun mentioned this pull request Dec 16, 2017
Merged
@amaitland
Copy link
Member

Merged with commit a4a8434

@amaitland amaitland closed this Dec 18, 2017
@amaitland amaitland reopened this Dec 18, 2017
@amaitland
Copy link
Member

Reverted changes with commit d654b9b

Switching to the Minimal dist won't be possible just yet as it doesn't contain a debug version of libcef.dll, so will have to revisit later.

@merceyz merceyz closed this Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Build.ps1: Use Minimal Distribution instead of Standard Distribution
4 participants
@merceyz @perlun @japj @amaitland